Skip to content

fix(security): bulk 写谓词守卫按调用者谓词判定 + pre-image 读取去重(#3018 审查后续)#3053

Merged
os-zhuang merged 1 commit into
mainfrom
claude/owner-id-security-fix-m36705
Jul 16, 2026
Merged

fix(security): bulk 写谓词守卫按调用者谓词判定 + pre-image 读取去重(#3018 审查后续)#3053
os-zhuang merged 1 commit into
mainfrom
claude/owner-id-security-fix-m36705

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

承接 PR #3018 那轮对抗式审查里记录、未在当时处理的两个加固点。

① bulk 写的反-oracle 谓词守卫改为顺序无关(潜在残留漏洞坐实并修复)

#2982 让 bulk update/delete 开始携带 opCtx.ast,于是它们首次进入 step 2.9 的反 filter-oracle 谓词守卫。该守卫的契约(见 predicate-guard.ts 文档注释)是对调用者自己的谓词判定——RLS / sharing 注入的过滤器合法地引用调用者不可读的字段(如 owner_id),不应被拒。

但对 bulk 写,守卫检查的是 opCtx.ast.where,而 plugin-sharing 的写分支可能已经owner_id 属主匹配 AND-合成进了 ast.where;两个中间件的注册顺序没有契约保证。核查结论:当前 verify 栈与生产栈都是 security 先于 sharing(security 外层),所以 2.9 在 sharing 注入前跑、目前是安全的(读路径同形、长期无 bug 也印证了这点)。但这是依赖注册顺序的隐性脆弱——一旦某对象把 owner_id 设为 FLS 不可读,且未来顺序被调整,该 private 对象上的所有 bulk 写会因注入的 owner_id 被 403。

修复:对 update/delete,守卫改看 opCtx.options.where(调用者未被改动的原始谓词),从此与中间件顺序无关,也绝不会把注入的属主/RLS 过滤器误判为探测。读路径不变(读的 seed 就是调用者查询原文,且本中间件在其自身 RLS 注入前运行)。

② pre-image 读取去重(维护性 + 少量性能)

同文件里“按 id 读目标行”的模式在 ~5 个门里各自内联、形态略有分歧(#3018 审查的 Reuse/Efficiency 项)。

  • readRowById(object, id, context) 统一读取形态(fail-closed:引擎缺失 / id 为空 / 读抛错 → null,null 一律 DENY,绝不放行),回填到几个 provenance 门(package-managed / system-row / controlled-by-parent)。
  • getCallerPreImage,把属主锚点回显检查(3.5)与 RLS check 后像(3.6)读取的同一 (object, id, 调用者上下文) 行按操作记忆化为一次读取。安全:两门之间没有对该行的写入(驱动写在整个中间件链之后),pre-image 在本操作内稳定。

纯行为等价重构;读取形态从此不会在各站点漂移。

验证

  • 单测:plugin-security 463(+1 新:注入到 AST 的 owner_id 不再触发守卫;调用者自己过滤隐藏字段仍被拒)。既有全部门测试通过 → ② 去重行为等价。
  • dogfood:owner-anchor-and-bulk-writes / two-doors-permission / controlled-by-parent / showcase-invoice-cbp / showcase-private-owd 共 27 全绿。
  • 已加 changeset(plugin-security patch)。

关联

🤖 Generated with Claude Code


Generated by Claude Code

…dupe pre-image reads (#3018 review)

Two hardening follow-ups from the #3018 adversarial review.

① Predicate guard order-independence for writes. #2982 made bulk update/delete
carry opCtx.ast, first bringing them under the step-2.9 anti-oracle guard. That
guard is documented to inspect the CALLER's own predicate (RLS/sharing filters
legitimately reference caller-unreadable fields like owner_id), but for a bulk
write it inspected opCtx.ast.where — which plugin-sharing (a sibling middleware
of unguaranteed order) may already have composed an owner_id match into. On an
object whose owner_id is FLS-hidden that could 403 a legitimate bulk write. The
guard now inspects opCtx.options.where (the caller's untouched predicate) for
update/delete, making it independent of middleware order and unable to mistake
an injected filter for a probe. Reads unchanged.

② Pre-image read dedup. A single readRowById helper (fail-closed: missing
engine / null id / thrown read → null, which always denies) now backs the
provenance gates, and a memoized getCallerPreImage collapses the owner-anchor
echo (3.5) and RLS check post-image (3.6) — the identical (object, id,
caller-context) row — into one read per operation. Pure refactor; the ~5 inlined
read sites can no longer drift.

Tests: plugin-security 463 (+1 new: injected owner_id in the AST does not trip
the guard, a caller filtering a hidden field still does); dogfood owner-anchor /
two-doors / CBP / invoice-cbp / private-owd green.
@vercel

vercel Bot commented Jul 16, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
spec Ready Ready Preview, Comment Jul 16, 2026 12:11pm

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling size/m labels Jul 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-security.

11 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/getting-started/cli.mdx (via @objectstack/plugin-security)
  • content/docs/permissions/access-recipes.mdx (via packages/plugins/plugin-security)
  • content/docs/permissions/authorization.mdx (via packages/plugins/plugin-security)
  • content/docs/permissions/explain.mdx (via @objectstack/plugin-security)
  • content/docs/permissions/permissions-matrix.mdx (via packages/plugins/plugin-security)
  • content/docs/permissions/sharing-rules.mdx (via @objectstack/plugin-security)
  • content/docs/plugins/index.mdx (via @objectstack/plugin-security)
  • content/docs/plugins/packages.mdx (via @objectstack/plugin-security)
  • content/docs/releases/implementation-status.mdx (via @objectstack/plugin-security)
  • content/docs/ui/audience-based-interfaces.mdx (via packages/plugins/plugin-security)
  • content/docs/ui/dashboards.mdx (via @objectstack/plugin-security)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@os-zhuang
os-zhuang marked this pull request as ready for review July 16, 2026 12:13
@os-zhuang
os-zhuang merged commit dee7feb into main Jul 16, 2026
16 checks passed
@os-zhuang
os-zhuang deleted the claude/owner-id-security-fix-m36705 branch July 16, 2026 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants